From c3fab20479ebd7fe1854f795ca4d96007caa032f Mon Sep 17 00:00:00 2001 From: tsteven4 <13596209+tsteven4@users.noreply.github.com> Date: Wed, 3 Mar 2021 09:53:39 -0700 Subject: [PATCH] Introduce RunMachine to manage gpsbabel process. (#699) * Introduce RunMachine to manage gpsbabel process. * fix comment * guard runmachine init state --- gui/CMakeLists.txt | 2 + gui/app.pro | 2 + gui/mainwindow.cc | 44 +++------ gui/processwait.cc | 191 +++++++++----------------------------- gui/processwait.h | 62 +++++-------- gui/runmachine.cc | 227 +++++++++++++++++++++++++++++++++++++++++++++ gui/runmachine.h | 109 ++++++++++++++++++++++ 7 files changed, 424 insertions(+), 213 deletions(-) create mode 100644 gui/runmachine.cc create mode 100644 gui/runmachine.h diff --git a/gui/CMakeLists.txt b/gui/CMakeLists.txt index 8aa576989..aaf5548cf 100644 --- a/gui/CMakeLists.txt +++ b/gui/CMakeLists.txt @@ -94,6 +94,7 @@ set(SOURCES optionsdlg.cc preferences.cc processwait.cc + runmachine.cc upgrade.cc version_mismatch.cc ) @@ -125,6 +126,7 @@ set(HEADERS optionsdlg.h preferences.h processwait.h + runmachine.h setting.h upgrade.h version_mismatch.h diff --git a/gui/app.pro b/gui/app.pro index f35531dc7..2746f05ce 100755 --- a/gui/app.pro +++ b/gui/app.pro @@ -92,6 +92,7 @@ SOURCES += mainwindow.cc SOURCES += optionsdlg.cc SOURCES += preferences.cc SOURCES += processwait.cc +SOURCES += runmachine.cc SOURCES += upgrade.cc SOURCES += version_mismatch.cc unix:!mac { @@ -124,6 +125,7 @@ HEADERS += mainwindow.h HEADERS += optionsdlg.h HEADERS += preferences.h HEADERS += processwait.h +HEADERS += runmachine.h HEADERS += setting.h HEADERS += upgrade.h HEADERS += version_mismatch.h diff --git a/gui/mainwindow.cc b/gui/mainwindow.cc index cbe885788..cb85045d5 100644 --- a/gui/mainwindow.cc +++ b/gui/mainwindow.cc @@ -43,6 +43,8 @@ #include // for QDesktopServices #include // for QIcon #include // for QImage +#include // for QTextCharFormat +#include // for QAbstractButton #include // for QApplication, qApp #include // for QCheckBox #include // for QDialogButtonBox @@ -70,7 +72,7 @@ #include "help.h" // for ShowHelp #include "optionsdlg.h" // for OptionsDlg #include "preferences.h" // for Preferences -#include "processwait.h" // for ProcessWaitDialog +#include "runmachine.h" // for RunMachine #include "upgrade.h" // for UpgradeCheck #include "version_mismatch.h" // for VersionMismatch @@ -862,36 +864,13 @@ bool MainWindow::isOkToGo() bool MainWindow::runGpsbabel(const QStringList& args, QString& errorString, QString& outputString) { - auto* proc = new QProcess(nullptr); QString name = "gpsbabel"; - proc->start(QApplication::applicationDirPath() + '/' + name, args); - auto* waitDlg = new ProcessWaitDialog(nullptr, proc); + QString program = QApplication::applicationDirPath() + '/' + name; + RunMachine runMachine(this, program, args); + int retStatus = runMachine.exec(); - if (proc->state() == QProcess::NotRunning) { - errorString = QString(tr("Process \"%1\" did not start")).arg(name); - return false; - } - - waitDlg->show(); - waitDlg->exec(); - bool retStatus = false; - if (waitDlg->getExitedNormally()) { - int exitCode = waitDlg->getExitCode(); - if (exitCode == 0) { - retStatus = true; - } else { - errorString = - QString(tr("Process exited unsuccessfully with code %1")) - .arg(exitCode); - retStatus = false; - } - } else { - retStatus = false; - errorString = waitDlg->getErrorString(); - } - outputString = waitDlg->getOutputString(); - delete proc; - delete waitDlg; + errorString = runMachine.getErrorString(); + outputString = runMachine.getOutputString(); return retStatus; } @@ -1038,7 +1017,14 @@ void MainWindow::applyActionX() } #endif } else { + QTextCharFormat defaultFormat = ui_.outputWindow->currentCharFormat(); + QTextCharFormat errorFormat = defaultFormat; + errorFormat.setForeground(Qt::red); + errorFormat.setFontItalic(true); + + ui_.outputWindow->setCurrentCharFormat(errorFormat); ui_.outputWindow->appendPlainText(tr("Error running gpsbabel: %1\n").arg(errorString)); + ui_.outputWindow->setCurrentCharFormat(defaultFormat); } } diff --git a/gui/processwait.cc b/gui/processwait.cc index 227591d43..3492bbe67 100644 --- a/gui/processwait.cc +++ b/gui/processwait.cc @@ -20,51 +20,30 @@ // USA. // //------------------------------------------------------------------------ -#include -#include -#include -#include -#include -#include -#include -#include -#include #include "processwait.h" -#include "appname.h" +#include // for QByteArray +#include // for Horizontal, WindowContextHelpButtonHint +#include // for QTextCursor, QTextCursor::End +#include // for QAbstractButton +#include // for QPushButton +#include // for QVBoxLayout + +#include // for abs +#include // for string + +#include "appname.h" // for appName -//------------------------------------------------------------------------ -QString ProcessWaitDialog::processErrorString(QProcess::ProcessError err) -{ - switch (err) { - case QProcess::FailedToStart: - return QString(tr("Process failed to start")); - break; - case QProcess::Crashed: - return QString(tr("Process crashed")); - break; - case QProcess::Timedout: - return QString(tr("Process timedout")); - break; - case QProcess::WriteError: - return QString(tr("Error while trying to write to process")); - break; - case QProcess::ReadError: - return QString(tr("Error while trying to read from process")); - break; - case QProcess::UnknownError: - default: - return QString(tr("Unknown process error")); - } - return QString(""); -} //------------------------------------------------------------------------ + ProcessWaitDialog::ProcessWaitDialog(QWidget* parent, QProcess* process): QDialog(parent), process_(process) { this->resize(400, 220); this->setWindowTitle(QString(appName) + tr(" ... Process GPSBabel")); + // turn off Help Button which can appear on windows as a '?'. + this->setWindowFlag(Qt::WindowContextHelpButtonHint, false); auto* layout = new QVBoxLayout(this); textEdit_ = new QPlainTextEdit(this); @@ -82,138 +61,60 @@ ProcessWaitDialog::ProcessWaitDialog(QWidget* parent, QProcess* process): btn->setText(tr("Stop Process")); layout->addWidget(buttonBox_); - connect(process, &QProcess::errorOccurred, - this, &ProcessWaitDialog::errorX); -// TODO: Qt6 combined the obsolete overloaded signal QProcess::finished(int exitCode) -// that required using qOverload. - connect(process, qOverload(&QProcess::finished), - this, &ProcessWaitDialog::finishedX); - connect(process, &QProcess::readyReadStandardError, - this, &ProcessWaitDialog::readyReadStandardErrorX); - connect(process, &QProcess::readyReadStandardOutput, - this, &ProcessWaitDialog::readyReadStandardOutputX); - connect(btn, &QAbstractButton::clicked, - this, &ProcessWaitDialog::stopClickedX); - exitStatus_ = QProcess::CrashExit; // Assume all errors are crashes for now. - - bufferedOut_ = ""; - - // - for (int i=0; i<=100; i+=2) { - progressVals_.push_back(i); - } - for (int i=98; i>0; i-=2) { - progressVals_.push_back(i); - } - progressIndex_ = progressVals_.size()/2; + connect(process_, &QProcess::readyReadStandardError, + this, [this]() { + appendToText(process_->readAllStandardError()); + }); + connect(process_, &QProcess::readyReadStandardOutput, + this, [this]() { + appendToText(process_->readAllStandardOutput()); + }); + connect(btn, &QAbstractButton::clicked, + this, &ProcessWaitDialog::reject); timer_ = new QTimer(this); timer_->setInterval(100); timer_->setSingleShot(false); connect(timer_, &QTimer::timeout, this, &ProcessWaitDialog::timeoutX); - stopCount_ = -1; - ecode_ = 0; timer_->start(); - errorString_ = ""; - -} - -//------------------------------------------------------------------------ -bool ProcessWaitDialog::getExitedNormally() -{ - return (errorString_.length() == 0); -} - -//------------------------------------------------------------------------ -QString ProcessWaitDialog::getErrorString() -{ - return errorString_; -} - -//------------------------------------------------------------------------ -int ProcessWaitDialog::getExitCode() -{ - return ecode_; -} - -//------------------------------------------------------------------------ -void ProcessWaitDialog::stopClickedX() -{ - process_->terminate(); } //------------------------------------------------------------------------ void ProcessWaitDialog::timeoutX() { - progressIndex_++; - int idx = progressIndex_ % progressVals_.size(); - progressBar_->setValue(progressVals_[idx]); - if (stopCount_ >=0) { - stopCount_++; - } - if (stopCount_ > 150) { - process_->kill(); - errorString_ = QString(tr("Process did not terminate successfully")); - timer_->stop(); - accept(); - } -} - -//------------------------------------------------------------------------ -void ProcessWaitDialog::errorX(QProcess::ProcessError err) -{ - errorString_ = processErrorString(err); - timer_->stop(); - accept(); + progressIndex_ = (progressIndex_ + 1) % 100; + int progress = 100 - 2 * std::abs(progressIndex_ - 50); + progressBar_->setValue(progress); } //------------------------------------------------------------------------ -void ProcessWaitDialog::finishedX(int exitCode, QProcess::ExitStatus es) -{ - ecode_ = exitCode; - if (es == QProcess::CrashExit) { - errorString_ = QString(tr("Process crashed while running")); - } - timer_->stop(); - accept(); -} - - -//------------------------------------------------------------------------ -// appendPlainText automatically puts in a new line with every call. That's -// why you have to buffer it, and only append when we get a real newline. +// appendPlainText automatically puts in a new line with every call, +// necessitating the need to to buffer it, and only append when we get a real +// newline. +// QPlainTextEdit also gets very slow when the accumulated plain text is +// extensive. +// Thus, we set the plain text to the last bit of output. // -void ProcessWaitDialog::appendToText(const char* ptr) +void ProcessWaitDialog::appendToText(const QByteArray& text) { - outputString_ += QString(ptr); - for (const char* cptr = ptr; *cptr != 0; cptr++) { - if (*cptr == '\r') { - continue; + static constexpr int textLimit = 16384; + + // It's faster if bufferedOut is a std::string compared to a QByteArray. + std::string bufferedOut; + bufferedOut.reserve(text.size()); + // Throw out carriage returns which cause double spacing. + for (const char ch : text) { + if (ch != '\r') { + bufferedOut += ch; } - if (*cptr == '\n') { - textEdit_->appendPlainText(QString::fromStdString(bufferedOut_)); - bufferedOut_ = ""; - continue; - } - bufferedOut_ += *cptr; } -} - + outputString_ += QByteArray::fromStdString(bufferedOut); -//------------------------------------------------------------------------ -void ProcessWaitDialog::readyReadStandardErrorX() -{ - QByteArray d = process_->readAllStandardError(); - appendToText(d.data()); + textEdit_->setPlainText(QString::fromLocal8Bit(outputString_.right(textLimit))); + textEdit_->moveCursor(QTextCursor::End); } //------------------------------------------------------------------------ -void ProcessWaitDialog::readyReadStandardOutputX() -{ - QByteArray d = process_->readAllStandardOutput(); - appendToText(d.data()); -} - void ProcessWaitDialog::closeEvent(QCloseEvent* event) { event->ignore(); diff --git a/gui/processwait.h b/gui/processwait.h index 37c8559c3..b9eaa8e7d 100644 --- a/gui/processwait.h +++ b/gui/processwait.h @@ -23,18 +23,19 @@ #ifndef PROCESSWAIT_H #define PROCESSWAIT_H +#include // for QByteArray +#include // for QObject +#include // for QProcess, QProcess::ExitStatus, QProcess::ProcessError +#include // for QString +#include // for QTimer +#include // for QCloseEvent +#include // for QDialog +#include // for QDialogButtonBox +#include // for QPlainTextEdit +#include // for QProgressBar +#include // for QWidget -#include -#include -#include -#include -using std::string; -using std::vector; -class QProgressBar; -class QPlainTextEdit; -class QDialogButtonBox; -class QTimer; //------------------------------------------------------------------------ class ProcessWaitDialog: public QDialog { @@ -42,45 +43,28 @@ class ProcessWaitDialog: public QDialog Q_OBJECT public: - // - ProcessWaitDialog(QWidget* parent, QProcess* process_); + ProcessWaitDialog(QWidget* parent, QProcess* process); - bool getExitedNormally(); - int getExitCode(); - QString getErrorString(); - QString getOutputString() const + QString getOutputString() { - return outputString_; + return QString::fromLocal8Bit(outputString_); } protected: - void closeEvent(QCloseEvent* event); - void appendToText(const char*); - QString processErrorString(QProcess::ProcessError err); - + void closeEvent(QCloseEvent* event) override; + void appendToText(const QByteArray& text); private slots: - void errorX(QProcess::ProcessError); - void finishedX(int exitCode, QProcess::ExitStatus); - void readyReadStandardErrorX(); - void readyReadStandardOutputX(); void timeoutX(); - void stopClickedX(); private: - vector progressVals_; - int progressIndex_; - int stopCount_; - string bufferedOut_; - QProcess::ExitStatus exitStatus_; - int ecode_; - QProcess* process_; - QProgressBar* progressBar_; - QPlainTextEdit* textEdit_; - QDialogButtonBox* buttonBox_; - QTimer* timer_; - QString errorString_; - QString outputString_; + int progressIndex_{0}; + QProcess* process_{nullptr}; + QProgressBar* progressBar_{nullptr}; + QPlainTextEdit* textEdit_{nullptr}; + QDialogButtonBox* buttonBox_{nullptr}; + QTimer* timer_{nullptr}; + QByteArray outputString_; }; #endif diff --git a/gui/runmachine.cc b/gui/runmachine.cc new file mode 100644 index 000000000..7fbe49c9e --- /dev/null +++ b/gui/runmachine.cc @@ -0,0 +1,227 @@ +/* + Copyright (C) 2021 Robert Lipe, gpsbabel.org + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + */ + +#include "runmachine.h" + +#include // for qDebug +#include // for QEventLoop +#include // for QNonConstOverload +#include // for QOverload, qOverload +#include // for QDialog + +#include "appname.h" // for appName + + +QString RunMachine::decodeProcessError(QProcess::ProcessError err) +{ + switch (err) { + case QProcess::FailedToStart: + return tr("Process failed to start"); + break; + case QProcess::Crashed: + return tr("Process crashed"); + break; + case QProcess::Timedout: + return tr("Process timedout"); + break; + case QProcess::WriteError: + return tr("Error while trying to write to process"); + break; + case QProcess::ReadError: + return tr("Error while trying to read from process"); + break; + case QProcess::UnknownError: + default: + return tr("Unknown process error"); + } +} + +RunMachine::RunMachine(QWidget* parent, + const QString& program, + const QStringList& args) : + QWidget(parent), program_(program), args_(args) +{ + process_ = new QProcess(this); + progress_ = new ProcessWaitDialog(this, process_); + // It is important that at least some of the fowarded signals are + // QueuedConnections to avoid reentrant use of RunMachine::execute which it + // is not designed for. + connect(process_, &QProcess::errorOccurred, + this, [this](QProcess::ProcessError error) { + execute(processErrorOccurred, + std::optional(error), + std::nullopt, + std::nullopt); + }, Qt::QueuedConnection); + // TODO: Qt6 combined the obsolete overloaded signal QProcess::finished(int exitCode) + connect(process_, qOverload(&QProcess::finished), + this, [this](int exitCode, QProcess::ExitStatus exitStatus) { + execute(processFinished, + std::nullopt, + std::optional(exitCode), + std::optional(exitStatus)); + }, Qt::QueuedConnection); + connect(process_, &QProcess::started, + this, [this]() { + execute(processStarted, + std::nullopt, + std::nullopt, + std::nullopt); + }, Qt::QueuedConnection); + connect(progress_, &ProcessWaitDialog::rejected, + this, [this]() { + execute(abortRequested, + std::nullopt, + std::nullopt, + std::nullopt); + }, Qt::QueuedConnection); +} + +int RunMachine::exec() +{ + open(); + + // block until complete. + QEventLoop loop; + connect(this, &RunMachine::finished, &loop, &QEventLoop::quit); + loop.exec(); + + return getRetStatus(); +} + +void RunMachine::open() +{ + execute(start, std::nullopt, std::nullopt, std::nullopt); +} + +void RunMachine::execute(SignalId id, + std::optional error, + std::optional exitCode, + std::optional exitStatus) +{ + if constexpr(debug) { + QDebug debugStream = qDebug(); + debugStream << "exec entering" << state_ << id; + if (error.has_value()) { + debugStream << *error; + } + if (exitStatus.has_value()) { + debugStream << *exitStatus; + } + if (exitCode.has_value()) { + debugStream << *exitCode; + } + } + + switch (state_) { + case init: + switch (id) { + case start: + process_->start(program_, args_); + state_ = starting; + break; + default: + if constexpr(debug) { + qDebug() << "signal" << id << "UNEXPECTED in init state!"; + } + break; + }; + break; + + case starting: + switch (id) { + case processErrorOccurred: + errorString_ = QString(tr("Process \"%1\" did not start")).arg(appName); + state_ = done; + emit finished(); + break; + case processStarted: + progress_->show(); + progress_->open(); + state_ = running; + break; + default: + if constexpr(debug) { + qDebug() << "signal" << id << "UNEXPECTED in starting state!"; + } + break; + }; + break; + + case running: + switch (id) { + case processErrorOccurred: + if constexpr(finishOnRunningError) { + progress_->accept(); + errorString_ = decodeProcessError(*error); + state_ = done; + emit finished(); + } + break; + case processFinished: + progress_->accept(); + if (*exitStatus == QProcess::NormalExit) { + if (*exitCode != 0) { + errorString_ = + QString(tr("Process exited unsuccessfully with code %1")) + .arg(*exitCode); + } + } else { + errorString_ = tr("Process crashed while running"); + } + state_ = done; + emit finished(); + break; + case abortRequested: + if constexpr(finishOnAbort) { + // To avoid a message from ~QProcess we need to close the process + // instead of killing it. + // QProcess: Destroyed while process (".../gpsbabel") is still running." + process_->close(); + errorString_ = tr("Process crashed while running"); + state_ = done; + emit finished(); + } else { + // Console applications on Windows that do not run an event loop, or + // whose event loop does not handle the WM_CLOSE message, can only be + // terminated by calling kill(). + process_->kill(); + } + break; + default: + if constexpr(debug) { + qDebug() << "signal" << id << "UNEXPECTED in running state!"; + } + break; + }; + break; + + case done: + break; + + default: + if constexpr(debug) { + qDebug() << "UNEXPECTED State!"; + } + break; + }; + if constexpr(debug) { + qDebug() << "exec leaving" << state_ << id; + } +} diff --git a/gui/runmachine.h b/gui/runmachine.h new file mode 100644 index 000000000..27b81698b --- /dev/null +++ b/gui/runmachine.h @@ -0,0 +1,109 @@ +/* + Copyright (C) 2021 Robert Lipe, gpsbabel.org + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + */ + +#ifndef RUNMACHINE_H +#define RUNMACHINE_H + +#include // for QObject +#include // for QProcess, QProcess::ExitStatus, QProcess::ProcessError, qt_getEnumName +#include // for QString +#include // for QStringList +#include // for QWidget + +#include // for optional, nullopt + +#include "processwait.h" // for ProcessWaitDialog + + +class RunMachine : public QWidget +{ + Q_OBJECT + +public: + + /* Types */ + + enum SignalId { + start, + processErrorOccurred, + processStarted, + processFinished, + abortRequested + }; + Q_ENUM(SignalId) + + enum State { + init, + starting, + running, + done + }; + Q_ENUM(State) + + /* Special Member Functions */ + + RunMachine(QWidget* parent, const QString& program, const QStringList& args); + + /* Member Functions */ + + static QString decodeProcessError(QProcess::ProcessError err); + int exec(); + void open(); + QString getOutputString() + { + return progress_->getOutputString(); + } + QString getErrorString() + { + return errorString_; + } + bool getRetStatus() const + { + return errorString_.isEmpty(); + } + +Q_SIGNALS: + void finished(); + +private: + + /* Constants */ + + static constexpr bool debug = false; + static constexpr bool finishOnAbort = false; + static constexpr bool finishOnRunningError = false; + + /* Member Functions */ + + void execute(SignalId id, + std::optional error, + std::optional exitCode, + std::optional exitStatus); + + /* Data Members */ + + QProcess* process_{nullptr}; + ProcessWaitDialog* progress_{nullptr}; + State state_{init}; + QString program_; + QStringList args_; + QString errorString_; + +}; +#endif // RUNMACHINE_H -- 2.30.2